refactor!: catch up Exn designs with the upstream #2384
Conversation
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
There was a problem hiding this comment.
Thanks a lot, that's much appreciated!
All looks good, but it looks like deref is required for convenient access to the error, but I am open to alternative solutions that don't add bloat/inconvenience to the site of creation.
Edit: I set it back into draft so I get a signal when it's deemed ready for review/merge.
This reverts commit 9bbe26a.
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
This is correct. I revert the change to keep the current solution. I'll consider whether to add this |
gix-error/src/exn/impls.rs
Outdated
|
|
||
| /// Use the current exception the head of a chain, adding `errors` to its children. | ||
| #[track_caller] | ||
| pub fn chain_iter<T, I>(mut self, errors: I) -> Exn<E> |
There was a problem hiding this comment.
Perhaps change this to chain_all as well.
P.S. I'll start a PR on exn to add these two chain methods. In the initial use case of exn in ScopeDB, we can always know all the children when constructing the chain/tree, so we doubt whether to make children mutable after the chain has been established. But reading gix's use cases, it seems chains that adding children afterwards can be helpful.
There was a problem hiding this comment.
Perhaps change this to chain_all as well.
That sounds great, please feel free to do this here, otherwise I will do it in a follow-up.
This reverts commit 73cf39a.
Signed-off-by: tison <[email protected]>
There was a problem hiding this comment.
Now I know gix-error has to wrap once more the inner error into Untyped because downcast must have the type param matched. In this case, it seems to be inevitable.
There was a problem hiding this comment.
In theory, the into_inner() and into_box() can be removed as there is always gix_error::Error to hold these types.
Let's leave it as is for now, and I will consider removing them to avoid that double-box.
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
|
All tests passed. Welcome to drop another review. |
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
This is great, thanks so much! It's my pleasure to have you as contributor!
Also, extra-thanks for the cleanup :).
|
Oh, may I ask to squash this PR into one commit? |
Sure. This is what I typically expected - Since we rely on GitHub, we use PR to track the dev history and squash commits for an easy-reading main linear commit history.
Agree. I'd change the PR title now. |
tisonkun
changed the title
|
Sorry, I meant the commits. I can always squash via GH or rewrite history, but I'd put words into your mouth and destroy your signatures. If you don't mind, I can do that of course. |
Never mind. Please do that. |
2 participants
This follows #2352 (comment).
I push several changes in separate commits. If you'd like to handle them in multiple PRs, please let me know.